Stabilize SourceId hashes relative to the workspace
authorTyler Hall <tylerwhall@gmail.com>
Sun, 29 Jan 2017 22:55:44 +0000 (17:55 -0500)
committerTyler Hall <tyler.hall@lexmark.com>
Sun, 25 Jun 2017 21:45:37 +0000 (17:45 -0400)
Currently a crate from a path source will have its metadata hash
incorporate its absolute path on the system where it is built. This
always impacts top-level crates, which means that compiling the same
source with the same dependencies and compiler version will generate
libraries with symbol names that vary depending on where the workspace
resides on the machine.

For paths inside the Cargo workspace, hash their SourceId relative to
the root of the workspace. Paths outside of a workspace are still hashed
as absolute.

This stability is important for reproducible builds as part of a larger
build system that employs caching of artifacts, such as OpenEmbedded.
OpenEmbedded tightly controls all inputs to a build and its caching
assumes that an equivalent artifact will always result from the same set
of inputs. The workspace path is not considered to be an influential
input, however.

For example, if Cargo is used to compile libstd shared objects which
downstream crates link to dynamically, it must be possible to rebuild
libstd given the same inputs and produce a library that is at least
link-compatible with the original. If the build system happens to cache
the downstream crates but needs to rebuild libstd and the user happens
to be building in a different workspace path, currently Cargo will
generate a library incompatible with the original and the downstream
executables will fail at runtime on the target.

src/cargo/core/package_id.rs
src/cargo/core/source.rs
src/cargo/ops/cargo_rustc/context.rs
src/cargo/ops/cargo_rustc/mod.rs

index 4058eeaacc4aea3510e1ebdbc0dbc166d2367ec7..7035ef5db0f53b9a983784fa6bfe55068eb5abd5 100644 (file)
@@ -2,6 +2,7 @@ use std::cmp::Ordering;
 use std::fmt::{self, Formatter};
 use std::hash::Hash;
 use std::hash;
+use std::path::Path;
 use std::sync::Arc;
 
 use semver;
@@ -131,6 +132,20 @@ impl PackageId {
             }),
         }
     }
+
+    pub fn stable_hash<'a>(&'a self, workspace: &'a Path) -> PackageIdStableHash<'a> {
+        PackageIdStableHash(&self, workspace)
+    }
+}
+
+pub struct PackageIdStableHash<'a>(&'a PackageId, &'a Path);
+
+impl<'a> Hash for PackageIdStableHash<'a> {
+    fn hash<S: hash::Hasher>(&self, state: &mut S) {
+        self.0.inner.name.hash(state);
+        self.0.inner.version.hash(state);
+        self.0.inner.source_id.stable_hash(self.1, state);
+    }
 }
 
 impl fmt::Display for PackageId {
index e7a3ce4a2e84bc6f7cdfb7922ce9520570443466..98730e3f081831cff70d0bc71c16d1145f4cf98f 100644 (file)
@@ -1,7 +1,7 @@
 use std::cmp::{self, Ordering};
 use std::collections::hash_map::{HashMap, Values, IterMut};
 use std::fmt::{self, Formatter};
-use std::hash;
+use std::hash::{self, Hash};
 use std::path::Path;
 use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT};
@@ -297,6 +297,17 @@ impl SourceId {
         }
         self.inner.url.to_string() == CRATES_IO
     }
+
+    pub fn stable_hash<S: hash::Hasher>(&self, workspace: &Path, into: &mut S) {
+        if self.is_path() {
+            if let Ok(p) = self.inner.url.to_file_path().unwrap().strip_prefix(workspace) {
+                self.inner.kind.hash(into);
+                p.to_str().unwrap().hash(into);
+                return
+            }
+        }
+        self.hash(into)
+    }
 }
 
 impl PartialEq for SourceId {
@@ -417,7 +428,7 @@ impl Ord for SourceIdInner {
 // The hash of SourceId is used in the name of some Cargo folders, so shouldn't
 // vary. `as_str` gives the serialisation of a url (which has a spec) and so
 // insulates against possible changes in how the url crate does hashing.
-impl hash::Hash for SourceId {
+impl Hash for SourceId {
     fn hash<S: hash::Hasher>(&self, into: &mut S) {
         self.inner.kind.hash(into);
         match *self.inner {
index 64e2e5b9f25ca802a48cd9fb1b7511492575cd76..b42247293a43ef6e8392d817b016d11d01d96dbf 100644 (file)
@@ -407,7 +407,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         let name = unit.pkg.package_id().name();
         match self.target_metadata(unit) {
             Some(meta) => format!("{}-{}", name, meta),
-            None => format!("{}-{}", name, util::short_hash(unit.pkg)),
+            None => format!("{}-{}", name, self.target_short_hash(unit)),
         }
     }
 
@@ -426,6 +426,13 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         self.build_config.requested_target.as_ref().map(|s| &s[..])
     }
 
+    /// Get the short hash based only on the PackageId
+    /// Used for the metadata when target_metadata returns None
+    pub fn target_short_hash(&self, unit: &Unit) -> String {
+        let hashable = unit.pkg.package_id().stable_hash(self.ws.root());
+        util::short_hash(&hashable)
+    }
+
     /// Get the metadata for a target in a specific profile
     /// We build to the path: "{filename}-{target_metadata}"
     /// We use a linking step to link/copy to a predictable filename
@@ -459,7 +466,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
         // Unique metadata per (name, source, version) triple. This'll allow us
         // to pull crates from anywhere w/o worrying about conflicts
-        unit.pkg.package_id().hash(&mut hasher);
+        unit.pkg.package_id().stable_hash(self.ws.root()).hash(&mut hasher);
 
         // Add package properties which map to environment variables
         // exposed by Cargo
index 67bb834a271ab4372968247d05dc61105dca3960..6f2925740d94e6a39bd66409fd70ded2599da06f 100644 (file)
@@ -12,7 +12,7 @@ use core::{Package, PackageId, PackageSet, Target, Resolve};
 use core::{Profile, Profiles, Workspace};
 use core::shell::ColorChoice;
 use util::{self, ProcessBuilder, machine_message};
-use util::{Config, internal, profile, join_paths, short_hash};
+use util::{Config, internal, profile, join_paths};
 use util::errors::{CargoResult, CargoResultExt};
 use util::Freshness;
 
@@ -791,7 +791,7 @@ fn build_base_args(cx: &mut Context,
             cmd.arg("-C").arg(&format!("extra-filename=-{}", m));
         }
         None => {
-            cmd.arg("-C").arg(&format!("metadata={}", short_hash(unit.pkg)));
+            cmd.arg("-C").arg(&format!("metadata={}", cx.target_short_hash(unit)));
         }
     }